-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Autocreate signing KV #317
base: main
Are you sure you want to change the base?
Conversation
This PR is currently under "semi-active" development and is expected to be "ready for review" later this week. I’d prefer to keep it open in the meantime, and I warmly welcome any comments, suggestions, or feedback—your input is greatly appreciated. The reason it has remained in draft status for so long is due to a slight shift in priorities and some challenges with testing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should not be three commits like these, first doing one thing and then reverting it. Likely the correct thing to do is to combine them to one commit. In any case they should be reworked.
We can rework or squash. Either way is fine for me. |
github action checks fail due to terraform formatting issues. |
ACK. Will do. Thank you! |
f828ff8
to
a21debc
Compare
Two questions/comments after testing the changes from this PR: (1) This PR adds the signing keyvault creation, but the added certificates are not used anywhere, right? What's the plan on actually using these certificates later on? (2) After the changes from this PR, the infra can not be destroyed following the instructions from https://github.com/tiiuae/ghaf-infra/tree/main/terraform#destroying-ghaf-infra-environment or by using the
I believe you want to add |
I agree, please squash the commits. |
Good point. Will fix & retest. Thanks! |
Just out of curiosity and "for quality and training purposes", what is the difference between manually squashing them now and squashing them before merge by "Squash and Merge"? |
I've never used "Squash and Merge", but some people claim that it doesn't allow manual editing of the commit message, or at least one easily accidentally go by the commit message automatically combined from the squashed commits. Other than that, I'd prefer to see final commit already before hitting Merge for it. |
Ok. Sure. I will manually squash it then. |
- create KV - Add 2 self-signed certificates Signed-off-by: Aleksandr Tserepov-Savolainen <[email protected]>
a21debc
to
7941e97
Compare
Automatically create signing KV and 2 self-signed certificates inside it.
This should serve as a base for the automatic keyvault creation and also provide signing capabilities for testing.